Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

copy NearCacheEntry values as-if they had been materialized from cache #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

davidAtInleague
Copy link

Goal here being to not share live object refs to the underlying "to-be-cached" objects across NearCacheEntry objects

resolves #13

@MordantWastrel
Copy link

MordantWastrel commented Sep 25, 2023

@zspitzer any chance we can get some traction on this?

@zspitzer
Copy link
Member

I've asked @michaeloffner to review

any chance of adding some tests?

@MordantWastrel
Copy link

It looks like a day or two to get the current test suite running and add a test for this. We can find time for that but probably not until later this year.

@MordantWastrel
Copy link

@zspitzer We've signed off on a day or so of work to get some tests in. Once that's done we're hopeful this can get merged in as otherwise we will need to fork this and build our own extension or use Ortus'. I think there is at least one other dev in the community who has done that so it might be worth looking to see whether there is interest in recruiting additional community members to help out with this extension (if it is indeed an official Lucee extension) or if it's more 'something the folks at LAS needed and helpfully decided to release but otherwise don't have time to maintain' -- either way is fine but it'd help us to know how you all look at it!

Thanks as always!

…all"

adds hook to inject thread delays for testing purposes
With goal being not sharing live object refs to the underlying
"to-be-cached" objects across NearCacheEntry objects

fixes test introduced in prior commit

resolves lucee#13
@davidAtInleague
Copy link
Author

I've asked @michaeloffner to review

any chance of adding some tests?

yes there is a test now; introduced in 0f7d5c5, where the problem is reproduced and so the test fails. The fix is introduced in the subsequent commit and the test turns green.

@davidAtInleague
Copy link
Author

I can't reproduce the failure against 5.4 locally, so maybe some additional as-yet-to-be-addressed race condition? The failing test appears to be testTwoLockedButReleasedOneLater. I've attached the generated test run's xml where that test passes, in case it's useful (had to rename to .txt for github to accept it, ha). I didn't try against 6 because it looks like it has been red for a while.

junit-test-results-5.4.3.2.xml.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache pulls non-deterministically return a live object ref to some "other" object
3 participants